Skip to content

Align cresc and dim in palette#32963

Merged
mike-spa merged 1 commit intomusescore:masterfrom
mike-spa:alignCrescDimInPalette
Apr 9, 2026
Merged

Align cresc and dim in palette#32963
mike-spa merged 1 commit intomusescore:masterfrom
mike-spa:alignCrescDimInPalette

Conversation

@mike-spa
Copy link
Copy Markdown
Contributor

@mike-spa mike-spa commented Apr 9, 2026

Palette elements are drawn centered on the cell by their bounding box, so they are not aligned at the baseline. See for instance also mp and mf.

Here dim has a taller bounding box because of the d and i ascenders, so it's expected that the baseline doesn't match with cresc. I don't know how they were aligned before. We could do fancy stuff using the font x-height etc but it's frankly not worth it, so I've just adjusted the offset.

Resolves: #32909

@mike-spa
Align cresc and dim in palette
4e48584 
Palette elements are drawn centered on the cell by their bounding box, so they are not aligned at the baseline. See for instance also mp and mf.

Here dim has a taller bounding box because of the d and i ascenders, so it's expected that the baseline doesn't match with cresc. I don't know how they were aligned before. We could do fancy stuff using the font x-height etc but it's frankly not worth it, so I've just adjusted the offset.
@mike-spa mike-spa requested a review from miiizen April 9, 2026 08:09
@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 9, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: c51625af-166d-4023-9282-d050e876755a

📥 Commits

Reviewing files that changed from the base of the PR and between 7c68382 and 4e48584.

📒 Files selected for processing (1)
  • src/palette/internal/palettecreator.cpp
📝 Walkthrough

Walkthrough

This change modifies the PaletteCreator::newDynamicsPalette method in the MuseScore palette creator component. The offset assignment logic for Hairpin elements was restructured to use explicit per-type selection instead of grouped ternary operators. The CRESC_LINE type now explicitly receives QPointF(1, 0.25), DIM_LINE receives QPointF(1, 0.0), and all other hairpin types default to QPointF(0, 0). The magnitude calculation and overall control flow remain unmodified.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately describes the main change: adjusting the alignment of cresc and dim elements in the palette to fix the vertical misalignment issue.
Description check ✅ Passed The PR description explains the problem (bounding box centering), why dim appears misaligned (ascenders), and the solution (offset adjustment). However, it lacks the required metadata like issue reference in standard format and verification checkboxes.
Linked Issues check ✅ Passed The code changes directly address issue #32909 by adjusting palette element offsets to align cresc and dim vertically, matching the observed misalignment regression described in the issue.
Out of Scope Changes check ✅ Passed All changes are focused on fixing the alignment issue in PaletteCreator::newDynamicsPalette; no extraneous modifications were introduced.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

@davidstephengrant
Copy link
Copy Markdown
Contributor

davidstephengrant commented Apr 9, 2026

@mike-spa Tested and approved on Ubuntu 24.04.4 LTS.

@mike-spa mike-spa merged commit 104ad0e into musescore:master Apr 9, 2026
13 checks passed
mike-spa added a commit to mike-spa/MuseScore that referenced this pull request Apr 9, 2026
@mike-spa
Merge pull request musescore#32963 from mike-spa/alignCrescDimInPalette
1874305
@mike-spa mike-spa mentioned this pull request Apr 9, 2026
Merged
RomanPudashkin added a commit that referenced this pull request Apr 9, 2026
@RomanPudashkin
Merge pull request #32968 from mike-spa/portStuff
c89b2a9 
Port #32963
@coderabbitai coderabbitai bot mentioned this pull request Apr 13, 2026
Open
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@miiizen miiizen miiizen approved these changes

Assignees

@mike-spa mike-spa

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

cresc. and dim. aren't vertically aligned in dynamics palette

3 participants

@mike-spa @davidstephengrant @miiizen